Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-44713: [Python] Add support for Decimal32 and Decimal64 types #44882

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Nov 28, 2024

Rationale for this change

Arrow C++ and the Arrow specification now support 32-bit and 64-bit decimal types...pyarrow should too!

What changes are included in this PR?

Added type, array, and scalar bindings.

Are these changes tested?

Working on it!

Are there any user-facing changes?

Yes! (Working on completing documentation)

Copy link

⚠️ GitHub issue #44713 has been automatically assigned in GitHub to PR creator.

@ianmcook
Copy link
Member

ianmcook commented Nov 29, 2024

Please add the concrete classes for the scalars to the list under the Scalars heading in arrays.rst.

And after you add the is_ methods, please add those under the Type Checking heading in datatypes.rst.

Thanks!

@paleolimbot
Copy link
Member Author

Current battles (so I remember them Monday!):

I'm missing some member definition for the use-facing type classes:

import pyarrow as pa

pa.decimal128(2, 0).precision
#> 2
pa.decimal256(2, 0).precision
#> 2
pa.decimal32(2, 0).precision
#> AttributeError: 'pyarrow.lib.Decimal32Type' object has no attribute 'precision'

There's no cast implemented from "float" to "decimal32/64", which at least some of the tests depend on:

pyarrow.lib.ArrowNotImplementedError: Unsupported cast to decimal32(7, -8) from float

@@ -180,7 +180,8 @@ def print_entry(label, value):
ListViewType, LargeListViewType,
MapType, UnionType, SparseUnionType, DenseUnionType,
TimestampType, Time32Type, Time64Type, DurationType,
FixedSizeBinaryType, Decimal128Type, Decimal256Type,
FixedSizeBinaryType,
Decimal32Type, Decimal64Type, Decimal128Type, Decimal256Type,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to add Decimal32Array, Decimal64Array to line 220 and Decimal32Scalar, Decimal64Scalar to line 229

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Dec 11, 2024
Parameters
----------
precision : int
Must be between 1 and 9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should update to between 1 and 18

@@ -1900,7 +1900,9 @@ def test_fsl_to_fsl_cast(value_type):
FloatToDecimalCase = namedtuple('FloatToDecimalCase',
('precision', 'scale', 'float_val'))

decimal_type_traits = [DecimalTypeTraits('decimal128', pa.decimal128, 38),
decimal_type_traits = [DecimalTypeTraits('decimal32', pa.decimal32, 8),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be 9, not 8?

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants